-
Notifications
You must be signed in to change notification settings - Fork 67
Add publication quality Cooccurrence plot to figure generation scripts #639
Add publication quality Cooccurrence plot to figure generation scripts #639
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The figure looks good and the approach seems reasonable. With the FLAGS update, I think we need to update the README for the module as part of this PR.
figures/generate-figures.sh
Outdated
bash ${analyses_dir}/interaction-plots/01-create-interaction-plots.sh | ||
|
||
# Copy the main figure to final directory | ||
cp ${analyses_dir}/interaction-plots/plots/combined_top50.pdf pdfs/mutation_cooccurrence_figure.pdf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For manubot purposes, I think we may want a PNG copy as well but not entirely sure. From the Usage documentation about figures:
For images created by the manuscript authors that are hosted elsewhere on GitHub, we recommend using a versioned GitHub URL to embed figures, thereby preserving exact image provenance. When embedding SVG images hosted on GitHub, it's necessary to append ?sanitize=true to the raw.githubusercontent.com URL.
We can move forward with the PDF and find out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will just change back to PNG. Sad to lose resolution, but if manubot isn't going to support it, we will just have to live with it.
@@ -49,6 +49,14 @@ if [ "$ALL" -gt "0" ]; then | |||
fi | |||
|
|||
|
|||
# Get FLAG file and add header |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The README for this module should get updated to include a citation for FLAGS, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
Purpose/implementation Section
The goal of this PR is to add another publication quality figure (not necessarily with publication quality results) to the figures/ directory and figure generation scripts.
This partially fulfills goals of #571
What was your approach?
I did some minor refactoring of the figure generation scripts, including a name change.
For the cooccurrence plot, there are some further minor changes, notably excluding the to 50 commonly mutated genes as defined by the FLAGS data set.
What GitHub issue does your pull request address?
#571, in part
Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.
Which areas should receive a particularly close look?
Are the changes to the main figure generation script reasonable/acceptable?
Is the choice of 50 FLAGs genes to exclude reasonable, or should there be a different list of genes excluded?
Is there anything that you want to discuss further?
Colors probably still need to be updated with the common color pallete in #622
Is the analysis in a mature enough form that the resulting figure(s) and/or table(s) are ready for review?
Yes
Results
What types of results are included (e.g., table, figure)?
What is your summary of the results?
Reproducibility Checklist
Documentation Checklist
README
and it is up to date.analyses/README.md
and the entry is up to date.